build tests #21
Open
build tests #21
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gemtechd
reviewed
Jul 23, 2024
Owner
gemtechd
left a comment
gemtechd
left a comment
There was a problem hiding this comment.
See the comments and work further on the task
modules/ecs6-class/line.js
Outdated
| throw new Error("n is not valid!") | ||
| } | ||
| if ((!(point1 instanceof Point)) || (!(point2 instanceof Point))) { | ||
| throw new Error("the object not instance of 'Point'!") |
modules/ecs6-class/point.js
Outdated
| constructor({x=0, y=0}={}) { | ||
| constructor({ x = 0, y = 0 } = {}) { | ||
| if(typeof(x)!=="number" || typeof(y)!=="number" ){ | ||
| throw new Error('argument is not a number!') |
modules/geometry-calculation.js
Outdated
|
|
||
| const calculateDistance = (point1, point2) => { | ||
| if(point1===undefined || point2===undefined ){ | ||
| throw new Error('the function must get an arguments!') |
| } | ||
| if((!(line1 instanceof Line))||(!(line2 instanceof Line))){ | ||
| throw new Error("the arguments is not instance of 'Line'!") | ||
| } |
Owner
There was a problem hiding this comment.
don't throw an error: You can calculate it
| if((!(line1 instanceof Line))||(!(line2 instanceof Line))){ | ||
| throw new Error("the arguments is not instance of 'Line'!") | ||
| } | ||
| if (line1.slope === line2.slope) { |
Owner
There was a problem hiding this comment.
what happens if the slope is still undefined or the n is still undefined?
Owner
There was a problem hiding this comment.
don't throw an error: You can calculate it
test/modules/ecs6-class/line.test.js
Outdated
| describe('RETURN_THE_POINT_ON_X_ASIS', () => { | ||
| it('should return the slope', () => { | ||
| line.getPointOnXAsis() | ||
| expect(line.getPointByY(2)).toEqual({ x: 1, y: 2 }) |
Owner
There was a problem hiding this comment.
you are checking the getPointOnXAsis
your expect a result from a different function - getPointbyY
and the description is about something totally different...
test/modules/ecs6-class/line.test.js
Outdated
| describe('RETURN_THE_POINT_ON_Y_ASIS', () => { | ||
| it('', () => { | ||
| line.getPointOnYAsis() | ||
| expect(line.getPointByX(2)).toEqual({ x: 2, y: 3 }) |
Owner
There was a problem hiding this comment.
where is the description of the test?
Owner
There was a problem hiding this comment.
Missing test descriptions
Where are the mocks?
gemtechd
reviewed
Jul 28, 2024
Owner
gemtechd
left a comment
gemtechd
left a comment
There was a problem hiding this comment.
See the comments
Build mocks in the tests
| @@ -6,16 +6,32 @@ let line = new Line({ point1, point2 }) | |||
| let line1 = new Line({ point1, point2 }) | |||
| const myline = new Line({}) | |||
Owner
There was a problem hiding this comment.
don't declare all parameters before the tests
do it inside each test apart
| describe('MOVE_VERTICAL', () => { | ||
| describe('POINT_CONSTRUCTOR', () => { | ||
| it('should check the point object', () => { | ||
| expect(mypoint.x).toBe(0) |
Owner
There was a problem hiding this comment.
declare the parameters inside each test
| let line4 = new Line({ point5, point6, slope: 1, n: 0 }) | ||
| let line5 = new Line({ point5, point4, slope: 1, n: 3 }) | ||
| let line6 = new Line({ point5, point4, slope: 1, n: 3 }) | ||
|
|
Owner
There was a problem hiding this comment.
don't declare parameters in the head of the module, each test gets its own object to test on it
| let line6 = new Line({ point5, point4, slope: 1, n: 3 }) | ||
|
|
||
| describe('CALCULATE_DISTANCE', () => { | ||
| it('return the sqrt for distance to point1 with point2', () => { |
Owner
There was a problem hiding this comment.
the description should be: should return the distance between two points. No need to explain the calculation
| it('', () => { | ||
| expect(calculateJunctionPoint(line1,line1)).toBe(true) | ||
| it('should return true if the slope and the n equal in line1 and line2', () => { | ||
| expect(calculateJunctionPoint(line1, line1)).toBe(true) |
Owner
There was a problem hiding this comment.
should return true when both lines are the same
| expect(calculateJunctionPoint(line1,line2)).toBe(false) | ||
|
|
||
| it('should return false if the slope equal and the n not in line1 and line2', () => { | ||
| expect(calculateJunctionPoint(line1, line2)).toBe(false) |
Owner
There was a problem hiding this comment.
should return false if both lines are parallel
| } | ||
| if((!(line1 instanceof Line))||(!(line2 instanceof Line))){ | ||
| throw new Error("the arguments is not instance of 'Line'!") | ||
| } |
Owner
There was a problem hiding this comment.
don't throw an error: You can calculate it
| if((!(line1 instanceof Line))||(!(line2 instanceof Line))){ | ||
| throw new Error("the arguments is not instance of 'Line'!") | ||
| } | ||
| if (line1.slope === line2.slope) { |
Owner
There was a problem hiding this comment.
don't throw an error: You can calculate it
Author
|
if the function need to get arguments and she didn't get i need to throw
error, no?
בתאריך יום א׳, 28 ביולי 2024 ב-3:26 מאת gemtechd <
***@***.***>:
… ***@***.**** commented on this pull request.
See the comments
Build mocks in the tests
------------------------------
In test/modules/ecs6-class/line.test.js
<#21 (comment)>:
> @@ -6,16 +6,32 @@ let line = new Line({ point1, point2 })
let line1 = new Line({ point1, point2 })
const myline = new Line({})
don't declare all parameters before the tests
do it inside each test apart
------------------------------
In test/modules/ecs6-class/point.test.js
<#21 (comment)>:
> @@ -1,8 +1,32 @@
const Point = require('../../../modules/ecs6-class/point')
let mypoint = new Point()
-describe('MOVE_VERTICAL', () => {
+describe('POINT_CONSTRUCTOR', () => {
+ it('should check the point object', () => {
+ expect(mypoint.x).toBe(0)
declare the parameters inside each test
------------------------------
In test/modules/geometry-calculation.test.js
<#21 (comment)>:
> -
-let line1 = new Line({point1, point2,slope:2,n:3})
-let line2 = new Line({point3, point4,slope:2,n:5})
-let line3 = new Line({point3, point4,slope:4,n:3})
-let line4 = new Line({point5, point6,slope:1,n:0})
-let line5 = new Line({point5, point4,slope:1,n:3})
-let line6 = new Line({point5, point4,slope:1,n:3})
-
-
-
+let line1 = new Line({ point1, point2, slope: 2, n: 3 })
+let line2 = new Line({ point3, point4, slope: 2, n: 5 })
+let line3 = new Line({ point3, point4, slope: 4, n: 3 })
+let line4 = new Line({ point5, point6, slope: 1, n: 0 })
+let line5 = new Line({ point5, point4, slope: 1, n: 3 })
+let line6 = new Line({ point5, point4, slope: 1, n: 3 })
don't declare parameters in the head of the module, each test gets its own
object to test on it
------------------------------
In test/modules/geometry-calculation.test.js
<#21 (comment)>:
> -
-let line1 = new Line({point1, point2,slope:2,n:3})
-let line2 = new Line({point3, point4,slope:2,n:5})
-let line3 = new Line({point3, point4,slope:4,n:3})
-let line4 = new Line({point5, point6,slope:1,n:0})
-let line5 = new Line({point5, point4,slope:1,n:3})
-let line6 = new Line({point5, point4,slope:1,n:3})
-
-
-
+let line1 = new Line({ point1, point2, slope: 2, n: 3 })
+let line2 = new Line({ point3, point4, slope: 2, n: 5 })
+let line3 = new Line({ point3, point4, slope: 4, n: 3 })
+let line4 = new Line({ point5, point6, slope: 1, n: 0 })
+let line5 = new Line({ point5, point4, slope: 1, n: 3 })
+let line6 = new Line({ point5, point4, slope: 1, n: 3 })
describe('CALCULATE_DISTANCE', () => {
it('return the sqrt for distance to point1 with point2', () => {
the description should be: should return the distance between two points.
No need to explain the calculation
------------------------------
In test/modules/geometry-calculation.test.js
<#21 (comment)>:
> })
})
})
describe('CALCULATE_JUNCTION_POINT', () => {
- it('', () => {
- expect(calculateJunctionPoint(line1,line1)).toBe(true)
+ it('should return true if the slope and the n equal in line1 and line2', () => {
+ expect(calculateJunctionPoint(line1, line1)).toBe(true)
should return true when both lines are the same
------------------------------
In test/modules/geometry-calculation.test.js
<#21 (comment)>:
> })
-
- it('', () => {
- expect(calculateJunctionPoint(line1,line2)).toBe(false)
+
+ it('should return false if the slope equal and the n not in line1 and line2', () => {
+ expect(calculateJunctionPoint(line1, line2)).toBe(false)
should return false if both lines are parallel
------------------------------
In modules/geometry-calculation.js
<#21 (comment)>:
> const distance = Math.sqrt(distanceX + distanceY);
return distance;
}
const calculateJunctionPoint = (line1, line2) => {
+ if(line1===undefined || line2===undefined ){
+ throw new Error('the function must get an arguments!')
+ }
+ if((!(line1 instanceof Line))||(!(line2 instanceof Line))){
+ throw new Error("the arguments is not instance of 'Line'!")
+ }
don't throw an error: You can calculate it
------------------------------
In modules/geometry-calculation.js
<#21 (comment)>:
> const distance = Math.sqrt(distanceX + distanceY);
return distance;
}
const calculateJunctionPoint = (line1, line2) => {
+ if(line1===undefined || line2===undefined ){
+ throw new Error('the function must get an arguments!')
+ }
+ if((!(line1 instanceof Line))||(!(line2 instanceof Line))){
+ throw new Error("the arguments is not instance of 'Line'!")
+ }
if (line1.slope === line2.slope) {
don't throw an error: You can calculate it
------------------------------
On test/modules/geometry-calculation.test.js
<#21 (comment)>:
I'm waiting to see the mocks
—
Reply to this email directly, view it on GitHub
<#21 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BJ6LP7DLM2IN7FDFKBC467TZOQ3D7AVCNFSM6AAAAABLJ5G7YCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMBTGI2DONBSGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.